Port `#[cfg]` to the new attribute parsing infrastructure #143460

JonathanBrouwer · 2025-07-04T22:32:52Z

Ports #[cfg] to the new attribute parsing infrastructure for #131229 (comment)

I've split this PR into commits for reviewability, and left some comments to clarify things

petrochenkov · 2025-07-05T17:15:31Z

I don't understand what is the point of this, at high level.
cfg and cfg_attr are a sort of macro attributes, they are not lowered into anything and do not appear in HIR.
I didn't yet hear about any plans to extend the new infra to something beyond inert attributes.

petrochenkov · 2025-07-05T17:21:43Z

Or is this is going to be only about cfg predicates that live inside cfg or cfg_attr?
Those can indeed occur in some inert attributes too, for example link, in that case they need to be lowered and encoded in metadata.

JonathanBrouwer · 2025-07-05T17:55:34Z

@petrochenkov

@jdonszelmann might be able to explain better, she has been supporting me, but I'll try my best.

We already have some infrastructure for parsing attributes in the early stage, before the HIR, and this is already used for the repr attribute. The long term plan is to completely get rid of rustc_attr/attr/mod.rs, and the cfg attribute uses this so it also needs to be converted to a parsed attribute. I hope the change will be clearer when this PR is ready, it is still very much work in progress. There's a nice amount of code which I should be able to delete when this PR is ready.

Furthermore as you mentioned some attributes like link also need this change, I currently have a PR open for porting link which is blocked on this change.

bors · 2025-07-06T20:54:42Z

☔ The latest upstream changes (presumably #143526) made this pull request unmergeable. Please resolve the merge conflicts.

JonathanBrouwer

@rustbot ready
r? @jdonszelmann

JonathanBrouwer · 2025-07-07T07:21:33Z

compiler/rustc_attr_parsing/src/attributes/cfg_old.rs

@@ -0,0 +1,247 @@
+use rustc_ast::{LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit, NodeId};


This file is temporarily still needed for the following things: #[doc], #[link], cfg!(), #[cfg_attr].
In the future this file, as well as a few others, should no longer be needed, but making these attributes use the new cfg parser is sadly not easily possible without parsing the entire attributes.

As well as this file, there is also the Cfg type in rustdoc and all errors associated with this file. All in all this PR could have a significantly negative diff (all of this is like ~600 lines of code) if not for this

JonathanBrouwer · 2025-07-07T07:22:48Z

compiler/rustc_expand/src/config.rs

+    pub(crate) fn cfg_true(&self, attr: &Attribute, node: NodeId) -> EvalConfigResult {
+        // We need to run this to do basic validation of the attribute, such as that lits are valid, etc
+        // FIXME(jdonszelmann) this should not be necessary in the future
+        match validate_attr::parse_meta(&self.sess.psess, attr) {


Sadly this is needed, @jdonszelmann are you aware of this / do you have any plans to fix this?
We're also still parsing all the other attributes using both the old and new parser for this, probably not performance sensitive but a bit weird

JonathanBrouwer · 2025-07-07T07:23:23Z

compiler/rustc_error_codes/src/error_codes/E0536.md

-pub fn main() {}
+pub fn main() {
+    if cfg!(not()) { }
+}


This error code will not be needed in the future (as wel as many others), but needed a new example for now

JonathanBrouwer · 2025-07-07T07:25:39Z

compiler/rustc_attr_parsing/src/context.rs

+    /// Whether to emit errors or delay them as a bug
+    /// For most attributes, the attribute will be parsed again in the `Late` stage and in this case the errors should be delayed
+    /// But for some, such as `cfg`, the attribute will be removed before the `Late` stage so errors must be emitted
+    pub emit_errors: bool,


This change is needed because #[cfg] will not be parsed in the Late stage, so we need to emit all errors/warnings now

rustbot · 2025-07-07T07:26:49Z

jdonszelmann is not on the review rotation at the moment.
They may take a while to respond.

rustbot · 2025-07-07T07:26:53Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_data_structures

cc @jdonszelmann

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

JonathanBrouwer · 2025-07-07T07:28:33Z

tests/ui/cfg/diagnostics-reexport.stderr

   |
 LL |     #[cfg(false)]
-   |     ^^^^^^^^^^^^^
+   |           ^^^^^


I made a small change here, this error will now always point to the exact part of the cfg attribute that makes the attribute false

compiler/rustc_attr_parsing/src/context.rs

compiler/rustc_attr_parsing/src/lib.rs

compiler/rustc_expand/src/config.rs

bors · 2025-07-10T17:51:38Z

☔ The latest upstream changes (presumably #143731) made this pull request unmergeable. Please resolve the merge conflicts.

JonathanBrouwer · 2025-07-11T09:30:31Z

compiler/rustc_attr_parsing/src/context.rs

+        features: Option<&'sess Features>,
+        emit_errors: bool,
+        parse_fn: fn(cx: &mut AcceptContext<'_, '_, Early>, item: &ArgParser<'_>) -> T,
+        template: &AttributeTemplate,


@jdonszelmann Instead of calling the general attribute parser I now call this function. This function takes a parse_fn and a template (sadly needed :c) and calls that specific parser. I think this is approximately what you wanted right?

@rustbot ready

yea, in fact I think we can migrate more to this system. nice!

bors · 2025-07-14T08:36:51Z

☔ The latest upstream changes (presumably #143779) made this pull request unmergeable. Please resolve the merge conflicts.

JonathanBrouwer · 2025-07-15T06:35:10Z

@rustbot author
I think I know what I did wrong

JonathanBrouwer · 2025-07-15T07:27:31Z

tests/ui/check-cfg/compact-names.rs

@@ -13,4 +13,8 @@ pub fn expected() {}
 //~^ WARNING unexpected `cfg` condition name
 pub fn unexpected() {}

+#[cfg(target(os = "windows", architecture = "arm"))]
+//~^ WARNING unexpected `cfg` condition name


The problem was that I implemented evaluation of all() and any() as short-circuiting.
This is not correct because during evaluation, sometimes lints are emitted (see https://github.com/JonathanBrouwer/rust/blob/7a7c74ad89df9f87824fa17fbbe0448d9ab6f7cc/compiler/rustc_attr_parsing/src/attributes/cfg.rs#L233). We should emit these lints, even after an earlier condition already failed.

The only test that tests this apparently is the unexpected test above this comment, which happens to only test this for non-linux targets. I added a second test unexpected2 to also test this on linux targets.

@rustbot ready
r? @jdonszelmann

Could you add a test for this that's not windows only?

The one above unexpected, would work for everything but linux
I added one that works for everything but linux
I suppose I could've just use false but whatever :P

rustbot · 2025-07-15T07:28:36Z

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

jdonszelmann · 2025-07-15T08:11:52Z

@bors r+

bors · 2025-07-15T08:11:54Z

📌 Commit 7a7c74a has been approved by jdonszelmann

It is now in the queue for this repository.

bors · 2025-07-15T08:11:55Z

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

bors · 2025-07-15T08:38:16Z

⌛ Testing commit 7a7c74a with merge a9fb610...

bors · 2025-07-15T11:41:48Z

☀️ Test successful - checks-actions
Approved by: jdonszelmann
Pushing a9fb610 to master...

github-actions · 2025-07-15T11:44:44Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 7f2065a (parent) -> a9fb610 (this PR)

Test differences

Show 26 test diffs

26 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard a9fb6103b05c6ad6eee6bed4c0bb5a2e8e1024c6 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

aarch64-apple: 3820.7s -> 5959.4s (56.0%)
dist-aarch64-linux: 8199.1s -> 5748.6s (-29.9%)
x86_64-apple-1: 10077.7s -> 7094.5s (-29.6%)
aarch64-msvc-2: 4977.3s -> 6371.3s (28.0%)
pr-check-2: 2117.9s -> 2633.1s (24.3%)
i686-gnu-2: 5332.5s -> 6164.7s (15.6%)
i686-gnu-nopt-1: 7109.8s -> 8100.8s (13.9%)
x86_64-gnu-llvm-20-1: 3286.7s -> 3723.8s (13.3%)
i686-gnu-1: 7307.2s -> 8217.8s (12.5%)
x86_64-gnu-distcheck: 7940.2s -> 8807.9s (10.9%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 4, 2025

JonathanBrouwer force-pushed the cfg_parser branch from 2da5a4c to 433025a Compare July 6, 2025 18:51

This comment has been minimized.

Sign in to view

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 6, 2025

JonathanBrouwer force-pushed the cfg_parser branch 2 times, most recently from 8a1e8fb to 60e69b8 Compare July 7, 2025 07:15

JonathanBrouwer commented Jul 7, 2025

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 7, 2025

rustbot assigned jdonszelmann Jul 7, 2025

JonathanBrouwer marked this pull request as ready for review July 7, 2025 07:26

JonathanBrouwer commented Jul 7, 2025

View reviewed changes

jdonszelmann requested changes Jul 8, 2025

View reviewed changes

compiler/rustc_attr_parsing/src/context.rs Outdated Show resolved Hide resolved

compiler/rustc_attr_parsing/src/lib.rs Show resolved Hide resolved

compiler/rustc_expand/src/config.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2025

jdonszelmann mentioned this pull request Jul 8, 2025

Tracking issue: Attribute refactor #131229

Open

9 tasks

JonathanBrouwer force-pushed the cfg_parser branch from 60e69b8 to 8853da6 Compare July 11, 2025 09:28

JonathanBrouwer commented Jul 11, 2025

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 11, 2025

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 14, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2025

JonathanBrouwer added 6 commits July 15, 2025 09:01

Define datastructures for #[cfg] attribute, move StrippedCfgItem

a14baf1

Allow Early stage to emit errors

38dd6f5

Move cfg -> cfg_old

30f4a9c

Define attribute parser & config evaluator

6133c67

Changes to diagnostics

c7f5ddc

New example for E0536

7a7c74a

JonathanBrouwer force-pushed the cfg_parser branch from ff21207 to 7a7c74a Compare July 15, 2025 07:21

JonathanBrouwer commented Jul 15, 2025

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 15, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 15, 2025

bors merged commit a9fb610 into rust-lang:master Jul 15, 2025
12 checks passed

rustbot added this to the 1.90.0 milestone Jul 15, 2025

This was referenced Jul 15, 2025

Ports #[macro_use] and #[macro_escape] to the new attribute parsing infrastructure #143843

Open

Stabilize #[cfg(version(...))], take 2 #141766

Open

Fix incorrect cfg structured suggestion and make suggestion verbose #137773

Open

		@@ -0,0 +1,247 @@
		use rustc_ast::{LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit, NodeId};

Port #[cfg] to the new attribute parsing infrastructure #143460

Port #[cfg] to the new attribute parsing infrastructure #143460

Conversation

JonathanBrouwer commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jul 5, 2025

Uh oh!

petrochenkov commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

bors commented Jul 6, 2025

Uh oh!

JonathanBrouwer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jul 7, 2025

Uh oh!

rustbot commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Jul 14, 2025

Uh oh!

JonathanBrouwer commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jul 15, 2025

Uh oh!

jdonszelmann commented Jul 15, 2025

Uh oh!

bors commented Jul 15, 2025

Uh oh!

bors commented Jul 15, 2025

Uh oh!

bors commented Jul 15, 2025

Uh oh!

bors commented Jul 15, 2025

Uh oh!

Uh oh!

github-actions bot commented Jul 15, 2025

Test differences

Job duration changes

Uh oh!

Uh oh!

Port `#[cfg]` to the new attribute parsing infrastructure #143460

Port `#[cfg]` to the new attribute parsing infrastructure #143460

JonathanBrouwer commented Jul 4, 2025 •

edited

Loading

petrochenkov commented Jul 5, 2025 •

edited

Loading

JonathanBrouwer commented Jul 5, 2025 •

edited

Loading

JonathanBrouwer commented Jul 15, 2025 •

edited

Loading

JonathanBrouwer Jul 15, 2025 •

edited

Loading